Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

table: RenderTSV to render in tab-separated-values format #277

Merged
merged 2 commits into from
Oct 4, 2023

Conversation

rafiramadhana
Copy link
Contributor

Proposed Changes

  • Add RenderTSV() string to Writer interface
  • Add RenderTSV() implementation

Fixes #276 .

@jedib0t @dinhani

@coveralls
Copy link

coveralls commented Oct 1, 2023

Pull Request Test Coverage Report for Build 6409214047

  • 50 of 50 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 5994588594: 0.0%
Covered Lines: 3361
Relevant Lines: 3361

💛 - Coveralls

@jedib0t
Copy link
Owner

jedib0t commented Oct 1, 2023

Hey @rafiramadhana thanks a lot for contributing this feature. Code looks good, but I have one concern. I believe that columns in TSV format are not allowed to have \t and \n characters as it will break formatting. Can you figure out the best way to deal with them?

One option I'm thinking of is replacing \t with \\t and \n with \\n so that the reader can interpret it correctly, but I don't know if that is what is usually done.

@jedib0t
Copy link
Owner

jedib0t commented Oct 1, 2023

@rafiramadhana It looks like you can double-quote columns which contain \t and \n. For example:

CSV:

"a","b	c","d
e"

TSV:

a	"b	c"	"d
e"

Can you please make this change in the logic?

	if strings.ContainsAny(col, "\t\n") {
		out.WriteString(fmt.Sprintf("\"%s\"", col))
	} else {
		out.WriteString(col)
	}

@jedib0t jedib0t changed the title Add render tsv table: RenderTSV to render table in tab-separated-values format Oct 1, 2023
@jedib0t jedib0t changed the title table: RenderTSV to render table in tab-separated-values format table: RenderTSV to render in tab-separated-values format Oct 1, 2023
@rafiramadhana
Copy link
Contributor Author

rafiramadhana commented Oct 2, 2023

I believe that columns in TSV format are not allowed to have \t and \n characters as it will break formatting.

Yes, that is correct as mentioned in Wikipedia

Sry, I haven't implement it because I rarely use a TSV

@rafiramadhana It looks like you can double-quote columns which contain \t and \n.

Agree, I think we can start with this implementation of adding double-quote

@rafiramadhana
Copy link
Contributor Author

rafiramadhana commented Oct 2, 2023

@rafiramadhana It looks like you can double-quote columns which contain \t and \n.

Do we need to escape the double-quote if the columns have double-quote?

asd"fgh    qwe    zxc

"asd""fgh"   qwe   zxc

This is similar with what is done in CSV (rfc4180)

@jedib0t
Copy link
Owner

jedib0t commented Oct 3, 2023

@rafiramadhana It looks like you can double-quote columns which contain \t and \n.

Do we need to escape the double-quote if the columns have double-quote?

asd"fgh    qwe    zxc

"asd""fgh"   qwe   zxc

This is similar with what is done in CSV (rfc4180)

I'm not sure what the spec calls for. But pasting "asd""fgh" qwe zxc into Excel made it come out right. So I think you are on the right track. Let's go with this approach for now.

Add .tsv handling for tabs, newlines, and doubel quotes.
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 4, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

out.WriteRune('\t')
}

if strings.ContainsAny(col, "\t\n\"") || strings.Contains(col, " ") {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jedib0t It looks like \t gets replaced by " " (4 spaces) in render_init.go#46

So, that's why I'm adding strings.Contains(col, " ") here

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I'll have to refactor the original function. This may not be ideal behavior for the end-user.

@jedib0t jedib0t merged commit 17050b7 into jedib0t:main Oct 4, 2023
3 checks passed
@jedib0t
Copy link
Owner

jedib0t commented Oct 4, 2023

Thanks @rafiramadhana for your contribution. Will cut a tag with this fix in a day or two.

@jedib0t
Copy link
Owner

jedib0t commented Oct 5, 2023

Tag with this feature: https://github.com/jedib0t/go-pretty/releases/tag/v6.4.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Table - Allow RenderCsv to be formatted with tabs instead of comma
3 participants